Attempt to fix flaky smb tests #1597
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ap--
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi @martindurant
While working on the other PR I had a look at the smb tests and could reproduce the random failures locally. The failures that occurred were either connection failures (likely because the container wasn't up yet) or authentication errors.
I noticed, that every time the tests fail,
SMBFileSystem._connect()which is called in__init__errored 5 times, and was not able to register the session in smbclient.I traced the authentication errors, and it turns out that, if
_connect()fails to register a session initially, the later calls to smbclient inSMBFileSystemlack the username and password credentials because smbclient will fallback to a credential free session, if none is registered.In this PR I just added a setting to be able to configure the number of retries for registering a session, and I set it to a very high number in the tests. With this fix, I am unable to reproduce failures locally.
It might be worth considering if
_connect()should raise an explicit error if username is not None and the retries are exhausted. Because if I understand the smbclient behavior correctly, subsequent usage of smbclient in the methods ofSMBFileSystemwill then always raise an AuthenticationError.